Fix Galvanic Iteration - #6978
Conversation
|
Warning Review limit reached
Next review available in: 1 minute You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe semantic audit now recognizes descriptionless ChangesCopySpell semantic-audit coverage
Estimated code review effort: 3 (Moderate) | ~15–30 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
the
needs-maintainer
|
Maintainer update pushed at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/engine/src/game/coverage.rs`:
- Around line 9391-9403: Split the CR annotation in the coverage classification
comments: cite CR 707.5 for the clone-permanent “enter as a copy of” branch, and
retain CR 707.10 only for spell-copy behavior on the stack. Update the nearby
surface-class descriptions without changing the matching logic.
- Around line 9405-9408: Update the copy-text predicate in
line_matches_effect_type so the “copy target” branch only matches text
explicitly identifying a spell, while preserving the existing “that spell” and
“copy it” handling. Ensure lines describing copied activated abilities do not
match when another ability has Effect::CopySpell, and add a negative regression
covering that case using the full typed CopySpell coverage rules.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9e404885-c23c-44bd-b473-538ee079bc66
📒 Files selected for processing (1)
crates/engine/src/game/coverage.rs
|
Generated for head Parse changes introduced by this PR✓ No card-parse changes detected. |
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Current head 8fa4e946fd3aa8774d16faa4fe60668cf1f6cd3f has two verified coverage/provenance findings.
[MED] The CopySpell coverage heuristic can hide an unparsed non-spell copy clause behind a different ability. Evidence: crates/engine/src/game/coverage.rs:9405-9408 accepts any copy target, while :9436-9447 searches the whole face for any CopySpell. Why it matters: a face with a valid spell-copy ability can be marked covered even when another ability's copy target <non-spell> line was not parsed. Suggested fix: require a spell-specific source phrase/shape and add a two-ability negative regression (valid spell copy plus unparsed non-spell copy).
[LOW] The enter-as-copy branch cites the wrong rule. Evidence: coverage.rs:9390-9403 describes both enter-as-copy and spell-copy under CR 707.10. Verified rules: docs/MagicCompRules.txt:5629 (CR 707.5) governs entering as a copy; :5666 (CR 707.10) governs copies of spells/abilities. Why it matters: CR citations must accurately identify the implemented rule. Suggested fix: split the comments/citations by the two copy classes.
matthewevans
removed
the
needs-maintainer
matthewevans
left a comment
matthewevans
left a comment
There was a problem hiding this comment.
Verified current head 14c6215: CopySpell coverage now requires spell-specific wording, has a discriminating non-spell-copy regression, and the CR 707.5/707.10 citations are correctly separated. Required CI is green.
2 participants
Summary
Fixes a parse-fidelity defect on Galvanic Iteration.
Issue: audit-flagged: SilentDrop — part of the Oracle text is silently dropped from the parsed AST; identify the missing clause and restore it.
Files changed
CR references
Track
Developer
LLM
Model: claude-opus-4-8
Thinking: high
Tier: Frontier
Verification
cargo fmt --all— clean (exit 0)./scripts/check-parser-combinators.sh (Gate A)— clean — Gate G PASS + Gate A PASS + Family-D cross-product detector self-test PASS (10/10). Initial run exited 1 ONLY because the broken WindowsApps python3 stub (exit 126) is first on PATH; re-run with the working msys64 python3 (3.9.7) passes fully. Env limitation, not a failure.cargo clippy-strict— clean (exit 0, 0 errors, 0 warnings; finished in 5m08s)cargo test -p phase-engine— FAILED (exit 101): 18467 passed, 1 failed, 6 ignored. Sole failure is environmental (Windows path-separator) in unrelated multiplayer prompt-census infra; unrelated to Galvanic Iteration.cargo export-cards data --stats --sidecar-dir client/public && cp client/public/card-data.json data/card-data.json— clean (EXPORT_EXIT 0, CP_EXIT 0; 333 set files / 28154 cards; data/card-data.json refreshed against this branch's engine)cargo coverage— clean (exit 0); Galvanic Iteration supported:true gap_count:0 (Flashback + CreateDelayedTrigger both supported)cargo semantic-audit— clean (exit 0); 32699 audited, 284 with findings; Galvanic Iteration has 0 findings (absent from data/semantic-audit.json and .md)Scope Expansion
None. (Finding reproduced; fix confined to coverage.rs as planned. It correctly cleared the whole spell-copy class — 10 cards — which is intended class coverage, not scope growth.)
Validation Failures
See review/cross-check notes.
CI Failures
Summary by CodeRabbit
Bug Fixes
Tests